SW-5466: Support Velodyne and other point types in ouster-ros driver #216
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Samahu
changed the title
mtswisher
suggested changes
Oct 12, 2023
There was a problem hiding this comment.
Just made some typo changes :) How do you decide whether to designate a function as inline? I haven't seen the factory pattern in quite awhile so it was cool to see that solve this problem. Will probably need to dig into some more to really understand what is going on but LGTM :)
Addressed!
Typically for short one liner methods this could minimally increase the code size but avoid having to invoke the method which is always faster .. With modern compilers the inline keyword only serves as a suggestion, compilers could inline methods automatically .. and could also ignore the suggestion if the method body is too long (in this case one needs to use __forceinline).
Yeah, the factory pattern is definitely needed here. I will add more comments to the PR to explain what's going on and how could developers use existing code to implement other point types that are not included in this PR. |
twslankard
approved these changes
Oct 13, 2023
…the functions + Add a short describion to all functions + Refactor unit tests + Add new ElementGetSet unit test
Add description for the point_cloud_compose methods + refactor code and add compile time checks.
This was referenced Nov 7, 2023
…pagate error state
Samahu
deleted the
SW-5466-support-velodyne-point-type-in-the-ros-driver
branch
This was referenced Nov 14, 2023
AchmadFathoni
pushed a commit
to AchmadFathoni/ouster-ros
that referenced
this pull request
Feb 16, 2024
…uster-lidar#216) * Quick protoype of Velodyne point type * Add PointXYZIR point type + other major pcl point types * Include point meta functions and point transform by the ouster_ros namesapce * Wrap point meta functions with a namespace and use shorter names for the functions + * Add a seed test module for the point_cloud_compose methods + Add description for the point_cloud_compose methods + refactor code and add compile time checks. * Propagate error state, warn about potential incompatible profile, propagate error state * Add minimal documentation about the new `point_type` parameter.
3 tasks
EirikKolas
added a commit
to vortexntnu/ouster-lidar-ros2-driver
that referenced
this pull request
Mar 23, 2024
* SW-5607: rename package ouster_msgs to avoid package name conflict in ros index (ouster-lidar#244) * rename package ouster_msgs to avoid conflict name conflict in ros index * set ouster_sensor_msgs version number to match with ouster_ros package * SW-5466: Support Velodyne and other point types in ouster-ros driver (ouster-lidar#216) * Quick protoype of Velodyne point type * Add PointXYZIR point type + other major pcl point types * Include point meta functions and point transform by the ouster_ros namesapce * Wrap point meta functions with a namespace and use shorter names for the functions + * Add a seed test module for the point_cloud_compose methods + Add description for the point_cloud_compose methods + refactor code and add compile time checks. * Propagate error state, warn about potential incompatible profile, propagate error state * Add minimal documentation about the new `point_type` parameter. * SW-5466: Support Velodyne point type in the ROS driver amendments (ouster-lidar#254) * Add support to control point_type through launch.xml files + * Add a note to CHANGELOG about the breaking change for ptp/utc time offset * SW-5623: Bump up ouster_client to 20231031 release (ouster-lidar#262) * Bump ouster-client to 2023103 release * fix: gracefully stop the driver when shutdown is requested. * ROS-196: laser scan from ros driver is not properly aligned with point cloud [humble] (ouster-lidar#203) * Apply destagger to laser scan + Add laser to RVIZ * Align LaserScan with the PointCloud * Apply proper pixel shift * Resolve the issue of zeroed laserscan on dual mode * Address an issue where LaserScan appeared different on FW prior to 2.4 * Fix the issue for odd numbers * List selected sensors on the main page + Update RVIZ config to highlight the 2D LaserScan. * Use timeout when waiting for packets to be proceed in case they don't come (ouster-lidar#293) * docs: fix spelling mistakes (ouster-lidar#296) --------- Co-authored-by: Ussama Naal <606033+Samahu@users.noreply.github.com> Co-authored-by: Andre Nguyen <andre-nguyen@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Related Issues & PRs
Summary of Changes
other common pcl point types.
Validation
point_typeparameter tonativeand verify that it reflects the same point cloud format that we get from the sensor.point_typeparameter toxyzand verify that the published point cloud format only hasxyzfields, regardless of the underlying sensor format.point_typeparameter toxyziand verify that the published point cloud format only hasxyzfields plus intensity, regardless of the underlying sensor format (you should get a warning when the low data mode is selected for this mode).point_typeparameter toxyzirand verify that the published point cloud format only hasxyzfields plus intensity plus ring, regardless of the underlying sensor format (you should get a warning when the low data mode is selected for this mode).